-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(controller, instrumenter): extract instrumentation logic #98
refactor(controller, instrumenter): extract instrumentation logic #98
Conversation
cmd/main.go
Outdated
Recorder: mgr.GetEventRecorderFor("dash0-controller"), | ||
Images: images, | ||
OTelCollectorBaseUrl: oTelCollectorBaseUrl, | ||
} | ||
go func() { | ||
time.Sleep(10 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this sleep?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sleep is a workaround for the following issue:
- We need to kick this (
InstrumentAtStartup
) off before we call mgr.Start(), becausemgr.Start()
blocks. - If
InstrumentAtStartup
runs right away, beforemgr.Start()
has actually run, it will throw an error, since the Kubernetesclient
that we get from themgr
is not started yet.
As the PR description explains, the aim of this PR is get rid of this arbitrary 10 second sleep -- by using a dedicated Kubernetes client that is independent from mgr
. I was working on that when you threw your latest PR over the wall :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sleep is gone now, as well as the goroutine -- which was the ultimate goal of the PR.
615b9a0
to
54ee987
Compare
54ee987
to
f44d111
Compare
Extract the controller's instrumentation logic into a separate module, called instrumentation/instrumenter. This allows running the InstrumentAtStartup task with a separate client and to move it into executeStartupTasks; getting rid of the arbitrary (and potentially problematic) 10 second wait, which we previously needed to wait until the manager had finished initializing its Kubernetes API client.
f44d111
to
8f0e756
Compare
Quality Gate passedIssues Measures |
Extract the controller's instrumentation logic into a separate module, called instrumentation/instrumenter. This will facilitate running the InstrumentAtStartup task with a separate client and ultimately move it into executeStartupTasks, getting rid of the arbitrary (and potentially problematic) 10 second wait.